Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FEATURE] Make alphabets trivial #3038

Merged
merged 2 commits into from
Oct 20, 2022
Merged

[FEATURE] Make alphabets trivial #3038

merged 2 commits into from
Oct 20, 2022

Conversation

eseiler
Copy link
Member

@eseiler eseiler commented Jun 24, 2022 •
edited
Loading

std::vector optimizes assignments to std::copy (basically memcpy) iff the value_type is trivial. If not, it will use std::unintialized_copy which will use placement-new and is around 2-3 times slower.

Method Container Alphabet Iterations before Iterations after
tag::assignment_operator std::vector seqan3::aa27 705 1853
tag::assignment_operator std::vector seqan3::dna4 741 1869
tag::assignment_operator std::vector seqan3::dna5 744 1879
tag::std_copy std::vector seqan3::aa27 1691 1871
tag::std_copy std::vector seqan3::dna4 1852 1800
tag::std_copy std::vector seqan3::dna5 1781 1865
tag::uninitialized_copy std::vector seqan3::aa27 711 1789
tag::uninitialized_copy std::vector seqan3::dna4 705 1856
tag::uninitialized_copy std::vector seqan3::dna5 727 1859
tag::assignment_operator std::vector seqan::AminoAcid 740 741
tag::assignment_operator std::vector seqan::Dna 733 726
tag::assignment_operator std::vector seqan::Dna5 747 727
tag::std_copy std::vector seqan::AminoAcid 1839 1811
tag::std_copy std::vector seqan::Dna 1766 1837
tag::std_copy std::vector seqan::Dna5 1830 1847
tag::uninitialized_copy std::vector seqan::AminoAcid 734 741
tag::uninitialized_copy std::vector seqan::Dna 742 739
tag::uninitialized_copy std::vector seqan::Dna5 731 735
tag::assignment_operator seqan::String seqan::AminoAcid 1835 1938
tag::assignment_operator seqan::String seqan::Dna 1819 1910
tag::assignment_operator seqan::String seqan::Dna5 1903 1944
tag::std_copy seqan::String seqan::AminoAcid 720 721
tag::std_copy seqan::String seqan::Dna 723 734
tag::std_copy seqan::String seqan::Dna5 723 726
tag::uninitialized_copy seqan::String seqan::AminoAcid 721 719
tag::uninitialized_copy seqan::String seqan::Dna 722 723
tag::uninitialized_copy seqan::String seqan::Dna5 719 728

@vercel
Copy link

vercel bot commented Jun 24, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
seqan3 ✅ Ready (Inspect) Visit Preview Oct 18, 2022 at 11:35AM (UTC)

@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jun 24, 2022
@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jun 25, 2022
@codecov
Copy link

codecov bot commented Jun 25, 2022 •
edited
Loading

Codecov Report

Base: 98.21% // Head: 98.21% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (d0011c9) compared to base (c3113fa).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3038      +/-   ##
==========================================
- Coverage   98.21%   98.21%   -0.01%     
==========================================
  Files         275      274       -1     
  Lines       12244    12204      -40     
==========================================
- Hits        12026    11986      -40     
  Misses        218      218
Impacted Files Coverage Δ
include/seqan3/alphabet/alphabet_base.hpp 100.00% <ø> (ø)
...clude/seqan3/alphabet/aminoacid/aminoacid_base.hpp 100.00% <ø> (ø)
include/seqan3/alphabet/aminoacid/aa20.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/aminoacid/aa27.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/structure/wuss.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/nucleotide/dna4.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/nucleotide/dna5.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/nucleotide/rna4.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/nucleotide/rna5.hpp 100.00% <0.00%> (ø)
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jun 25, 2022
@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jun 25, 2022
@eseiler eseiler force-pushed the misc/trivial_alphabet branch from 0632de1 to d678fe0 Compare June 25, 2022 15:42
@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jun 25, 2022
@eseiler eseiler force-pushed the misc/trivial_alphabet branch from d678fe0 to dcd6552 Compare October 17, 2022 11:02
@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Oct 17, 2022
@eseiler eseiler force-pushed the misc/trivial_alphabet branch from 72d9768 to 98a2510 Compare October 17, 2022 11:16
@seqan-actions seqan-actions added the lint [INTERNAL] signal for linting label Oct 17, 2022
@eseiler eseiler marked this pull request as ready for review October 17, 2022 11:17
@seqan-actions seqan-actions removed the lint [INTERNAL] signal for linting label Oct 17, 2022
@SGSSGene SGSSGene self-requested a review October 18, 2022 08:16
@SGSSGene SGSSGene mentioned this pull request Oct 18, 2022
Merged
4 tasks
@eseiler eseiler force-pushed the misc/trivial_alphabet branch from 98a2510 to 6931d95 Compare October 18, 2022 09:18
@eseiler eseiler changed the title [WIP] What if alphabets were trivial? [FEATURE] Make alphabets trivial Oct 18, 2022
@seqan-actions seqan-actions added the lint [INTERNAL] signal for linting label Oct 18, 2022
@eseiler
Copy link
Member Author

eseiler commented Oct 18, 2022 •
edited
Loading

Blocked by #3037

@seqan-actions seqan-actions removed the lint [INTERNAL] signal for linting label Oct 18, 2022
@eseiler eseiler force-pushed the misc/trivial_alphabet branch from 6931d95 to d0011c9 Compare October 18, 2022 11:32
@seqan-actions seqan-actions added the lint [INTERNAL] signal for linting label Oct 18, 2022
@seqan-actions seqan-actions removed the lint [INTERNAL] signal for linting label Oct 18, 2022
@eseiler eseiler requested a review from smehringer October 18, 2022 18:39
@eseiler eseiler enabled auto-merge October 19, 2022 08:56
SGSSGene
SGSSGene approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@SGSSGene SGSSGene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improvements look great!

@eseiler eseiler merged commit f71d4de into seqan:master Oct 20, 2022
@eseiler eseiler deleted the misc/trivial_alphabet branch October 20, 2022 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smehringer smehringer smehringer approved these changes

@SGSSGene SGSSGene SGSSGene approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@eseiler @SGSSGene @smehringer @seqan-actions